-
Notifications
You must be signed in to change notification settings - Fork 423
feat(multiagent): Add stream_async #961
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
src/strands/multiagent/graph.py
Outdated
Yields: | ||
Dictionary events containing graph execution information including: | ||
- MultiAgentNodeStartEvent: When a node begins execution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These types aren't exposed to customers, so we should either remove these docs or document the shape of the dictionaries being emited
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also list out the the new events to the PR description (similar to #788) along with the signatures of the new apis being added. This well help the PR be more akin to the spec of what's being proposed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we should either remove these docs AND document the shape of the dictionaries being emitted*
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have matched the implementation/documentation to agent. I will add additional docs as a followup CR on docs repo. I will also update the RP description to explain the events emitted.
src/strands/multiagent/graph.py
Outdated
try: | ||
event = await asyncio.wait_for(async_generator.__anext__(), timeout=timeout) | ||
yield event | ||
except StopAsyncIteration: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this always thrown at the end and thus part of normal execution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: seems like theres might be a more pythonic way to do this without the while
async with asyncio.timeout(timeout):
async for event in async_generator:
# because a failure would throw exception and code would not make it here | ||
ready_nodes.extend(self._find_newly_ready_nodes(current_batch)) | ||
|
||
async def _execute_nodes_parallel( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this follow the pattern of how tools are executed in parallel - I know we worked out some wrinkles/bugs with that, so it'd be ideal to not have to redo those
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, does this have to be in this PR? Can we do the streaming then add parallel execution as a different commit to simplify the review?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parallel calls should be in this PR as graph already supports them.
w.r.t. bugs, I have updated the logic similar to #954 . is there anything else?
src/strands/multiagent/graph.py
Outdated
start_event = MultiAgentNodeStartEvent( | ||
node_id=node.node_id, node_type="agent" if isinstance(node.executor, Agent) else "multiagent" | ||
) | ||
yield start_event.as_dict() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we do this at a higher level instead of in here? That way we can ensure this method is always returning TypedEvent
s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for below
src/strands/multiagent/graph.py
Outdated
wrapped_event = MultiAgentNodeStreamEvent(node.node_id, event) | ||
yield wrapped_event.as_dict() | ||
# Capture the final result event | ||
if "result" in event: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just do an isinstance
check here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not easily, because agent also translates it to a dict before returning the responses. https://github.com/strands-agents/sdk-python/blob/main/src/strands/agent/agent.py#L591
Is there a reason we decided to go this way instead of returning typed events?
src/strands/multiagent/swarm.py
Outdated
|
||
except Exception: | ||
logger.exception("node=<%s> | node execution failed", current_node.node_id) | ||
except Exception as e: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't we use an exception type for this? This seems hacky
status=Status.EXECUTING, | ||
task=task, | ||
total_nodes=len(self.nodes), | ||
edges=[(edge.from_node, edge.to_node) for edge in self.edges], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note related to this PR, but why doesn't GraphState take Iterable[GraphEdge] instead of edges: list[Tuple["GraphNode", "GraphNode"]] = field(default_factory=list)
Did GraphEdge come later?
src/strands/multiagent/graph.py
Outdated
try: | ||
event = await asyncio.wait_for(async_generator.__anext__(), timeout=timeout) | ||
yield event | ||
except StopAsyncIteration: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: seems like theres might be a more pythonic way to do this without the while
async with asyncio.timeout(timeout):
async for event in async_generator:
# because a failure would throw exception and code would not make it here | ||
ready_nodes.extend(self._find_newly_ready_nodes(current_batch)) | ||
|
||
async def _execute_nodes_parallel( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, does this have to be in this PR? Can we do the streaming then add parallel execution as a different commit to simplify the review?
src/strands/multiagent/swarm.py
Outdated
self, async_generator: AsyncIterator[dict[str, Any]], timeout: float, timeout_message: str | ||
) -> AsyncIterator[dict[str, Any]]: | ||
"""Wrap an async generator with timeout functionality.""" | ||
while True: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same nit as in graph
src/strands/multiagent/swarm.py
Outdated
logger.exception("node=<%s> | node execution failed", current_node.node_id) | ||
except Exception as e: | ||
# Check if this is a timeout exception | ||
if "timed out after" in str(e): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can we create a variable for this so we don't accidentally change the message in exception
- Update docstrings to match Agent's minimal style (use dict keys instead of class names) - Add isinstance checks for result event detection for type safety - Improve _stream_with_timeout to handle None timeout case - Add MultiAgentResultEvent for consistency with Agent pattern - Yield TypedEvent objects internally, convert to dict at API boundary - All 154 tests passing
- Remove unnecessary asyncio.gather() after event loop completion - Same issue as tool executor PR strands-agents#954 - By the time loop exits, all tasks have already completed - Gather was waiting for already-finished tasks (no-op) - All 154 tests passing
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Description
This PR adds streaming support to the Swarm and Graph multi-agent systems, enabling real-time event emission during multi-agent execution. This brings multi-agent systems to feature parity with the single Agent class streaming capabilities.
Key Changes
New Event Types (
src/strands/types/_events.py
):MultiAgentNodeStartEvent
: Emitted when a node begins executionMultiAgentNodeCompleteEvent
: Emitted when a node completes executionMultiAgentNodeStreamEvent
: Forwards agent events with node contextMultiAgentHandoffEvent
: Emitted during agent handoffs in Swarm (includes from_node, to_node, and message)Swarm Streaming (
src/strands/multiagent/swarm.py
):stream_async()
method that yields events during executioninvoke_async()
to usestream_async()
internally (maintains backward compatibility)Graph Streaming (
src/strands/multiagent/graph.py
):stream_async()
method for real-time event streaminginvoke_async()
to consumestream_async()
eventsTesting:
Benefits
Related Issues
Documentation PR
Type of Change
New feature
Testing
How have you tested the change?
tests/strands/multiagent/test_swarm.py
,tests/strands/multiagent/test_graph.py
)Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
hatch run prepare
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.